-
Notifications
You must be signed in to change notification settings - Fork 473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RegexArrayShapeMatcher - More precise non-empty-string and numeric-string #3249
Conversation
//cc @Seldaek would be great to have another pair of eyeballs on the test-expectations |
This pull request has been marked as ready for review. |
Thank you. |
1 similar comment
Thank you. |
Please send update to phpstan-nette :) |
if (preg_match('/Price: (£|€)?\d+/', $s, $matches, PREG_UNMATCHED_AS_NULL)) { | ||
assertType('array{0: string, 1?: string}', $matches); | ||
assertType('array{0: string, 1?: non-empty-string}', $matches); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given unmatched as null this should be 1: non-empty-string|null
IMO.
preg_match('/Price: (£|€)?\d+/', 'Price: 3', $matches, PREG_UNMATCHED_AS_NULL)
for ex yields:
array (size=2)
0 => string 'Price: 3' (length=8)
1 => null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is the php72 variant, which does not "correctly support" nulls in the PREG_UNMATCHED_AS_NULL
case (because php-src behaves differently).
see https://3v4l.org/No3Tf#v7.3.26
there is the identical test but for php7.4+ with the expectations you just shared:
https://github.com/phpstan/phpstan-src/pull/3249/files#diff-587efc95b2825fa3f959191a2c1dae92edda5ad901653ae174e58bc723152fa9R25
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry I missed that 🤦🏻♂️
Ok this is amazing :D I can't wait to try |
based on the regex ast we try to detect more precise string types, like non-empty-string and numeric-string.
I intentionally left out constant-string types for now to reduce complexity and learn from real world use-cases in the wild.